Codecov Report

Merging #247 into master will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #247   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           16        16           
  Lines          873       880    +7     
=========================================
+ Hits           873       880    +7     

I dived into the Flake8 and pytest source.

A Flake8Item inherits from File and Item, https://github.com/tholo/pytest-flake8/blob/e2461d12d82523db55f4d156d1c060da1e66b21e/pytest_flake8.py#L95:

class Flake8Item(pytest.Item, pytest.File):

From pytest, it looks like only Function Items have an obj: https://github.com/pytest-dev/pytest/blob/694fdc655436b6b2eb335d9019002ab81486e4d8/src/_pytest/python.py#L1420. An "Item" is an individual test invocation.

It seems we've conflated the concept of "node" with "item" in some cases, although in the report collector it is in fact an "Item". An FSCollector, when provided with a path, should return a "Module" (or Package). A "Module" is itself a PyCollector which collects all items in the module, creating items or collectors. The idea being that a Module's collect method may return a leaf node such as a Function(Item), or a non-leaf node such as a Class which itself would be a collector.

There is an example in pytest of a File which is also an Item: https://github.com/pytest-dev/pytest/blob/5034399d7acc4bd14fdad3d056a9abc2fde13863/testing/example_scripts/fixtures/custom_item/conftest.py#L4. So it seems there is official support by pytest of this use case. It's definitely strange though, since without any plugins/flake8 extensions, a File is usually a collector and would thus never end up in the collected items.

I believe the better fix would be to check that the Item is an instance of a Function node. Is there a use case where a snapshot would be asserted outside of a Function node?

Having trouble writing a test for this. I've tried:

from .utils import clean_output
def test_ignores_non_function_nodes(testdir):
    conftest = (
        """
        import pytest
        class CustomItem(pytest.Item, pytest.File):
            def __init__(self, *args, **kwargs):
                super().__init__(*args, **kwargs)
                self._nodeid += "::CUSTOM"
            def runtest(self):
                pass
        def pytest_collect_file(path, parent):
            return CustomItem(path, parent)
        """
    )
    testcase = (
        """
        def test_example(snapshot):
            assert snapshot == 1
        """
    )
    testdir.makepyfile(conftest=conftest)
    testdir.makepyfile(test_file=testcase)
    result = testdir.runpytest("-v", "--snapshot-update")
    result_stdout = clean_output(result.stdout.str())
    assert result.ret == 0
    assert "test_file.py::CUSTOM" in result_stdout

however it always passed.

It also seems the flake8 issue only happens 1 out of every 3 times, even with cache disabled. The behaviour alternates.

I can reproduce using:

       import pytest

        class CustomItem(pytest.Item, pytest.File):
            def __init__(self, *args, **kwargs):
                super().__init__(*args, **kwargs)
                self._nodeid += "::CUSTOM"

            def runtest(self):
                pass

        def pytest_collect_file(path, parent):
            return CustomItem(path, parent)

without pytest-flake8.

I've identified what's causing the flakiness. It depends entirely on the order we iterate through self.ran_items in the "unused" method of the report. If the valid function node is processed first, the test will complete successfully. If the CustomItem is processed first, it'll raise an error. This is because any short-circuits after the first function matches.

🎉 This PR is included in version 0.4.4 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀